Skip to content

Conversation

bspratt
Copy link
Member

@bspratt bspratt commented Sep 10, 2024

Reported by user Trevor

…ng added to molecules described only as a mass with no chemical formula.

Reported by user Trevor
@bspratt bspratt requested a review from nickshulman September 10, 2024 18:33
Copy link
Contributor

@nickshulman nickshulman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. This should probably be cherry picked back to 24.1

// Add in the "Na" of [M+Na] (or remove the 4H in [M-4H])
foreach (var pair in Composition)
{
if (resultDict.TryGetValue(pair.Key, out var count))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you wanted, you could replace these 17 lines with:

                resultDict.TryGetValue(pair.Key, out var count);
                count += pair.Value;
                if (count == 0)
                {
                    resultDict.Remove(pair.Key);
                }
                else
                {
                    resultDict[pair.Key] = count;
                }

@bspratt bspratt added the Cherry pick to release Merging PRs to master with this will create a new PR that CPs all commits to Skyline release branch label Sep 10, 2024
@bspratt bspratt merged commit 3040d17 into master Sep 10, 2024
10 checks passed
@bspratt bspratt deleted the Skyline/work/20240910_fix_apply_adducts_to_mass_only_molecules branch September 10, 2024 21:03
bspratt added a commit that referenced this pull request Sep 10, 2024
…ng added to molecules described only as a mass with no chemical formula. (#3151)

Reported by user Trevor

Backport from master commit 3040d17
bspratt added a commit that referenced this pull request Sep 10, 2024
* Fix a bug in m/z calculation where the mass of the adduct was not being added to molecules described only as a mass with no chemical formula. (#3151)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Cherry pick to release Merging PRs to master with this will create a new PR that CPs all commits to Skyline release branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants